Skip to content

PCSM-203 Implement copy of chunk distribution from source - #71

Open
rasika-chivate wants to merge 12 commits into
mainfrom
PCSM-203-Implement-copy-of-chunk-distribution-from-source
Open

PCSM-203 Implement copy of chunk distribution from source#71
rasika-chivate wants to merge 12 commits into
mainfrom
PCSM-203-Implement-copy-of-chunk-distribution-from-source

Conversation

@rasika-chivate

Copy link
Copy Markdown
Collaborator

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Documentation contradictions, unsupported guarantees, recovery ambiguity, and an invalid mongosh example remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Documents initial source chunk-layout copying for sharded migrations.

Changes:

  • Explains ranged and hashed shard-key behavior.
  • Adds recovery and layout verification guidance.
  • Covers differing shard counts and balancing behavior.
File summaries
File Description
docs/sharding.md Documents target chunk distribution during cloning.
Review details

Suppressed comments (8)

docs/sharding.md:57

  • “Every sharded collection” includes hashed-key collections, directly contradicting line 71, which says they are not pre-split. Restrict this automatic pre-split statement to ranged shard keys.
This runs automatically for every sharded collection, immediately after {{pcsm.short}} shards it on the target. There is no flag and nothing to configure.

docs/sharding.md:63

  • Preserving source boundaries does not guarantee roughly equal volume on every target shard. For example, one source chunk copied to a five-shard target can be assigned to only one shard. Describe the size-based assignment without promising a distribution that is mathematically impossible for low chunk counts.
| Ranged shard key | Different number from the source | The same chunk boundaries, with each target shard holding roughly the same volume of data. |

docs/sharding.md:79

  • The claim that every shard owns chunks is false when the collection has fewer chunks than the target has shards; preserving the same boundaries prevents creating enough chunks to satisfy it. State this as a balancing objective rather than a guarantee.
With different shard counts, the boundaries still come from the source, but the largest chunks are placed first, each on whichever target shard holds the least data at that point. Every shard ends up owning chunks and holding roughly the same volume. The estimate carries across collections, so the large chunks of several collections do not all collect on one shard.

docs/sharding.md:108

  • This is not valid mongosh JavaScript: a dot property cannot be the <collection> placeholder. Use getCollection() so readers can substitute any collection name and run the example.
db.getSiblingDB('<database>').<collection>.getShardDistribution()

docs/sharding.md:111

  • “Data on every shard” is not a valid success criterion when the copied source boundaries yield fewer chunks than target shards, because a chunk has only one owner. Document that empty target shards can be expected in this case instead of directing users to treat them as a failed layout.
Look for data on every shard rather than an exact match with the source, since counts differ even immediately after the clone and keep changing as the balancer works. For chunk counts per shard across the cluster, use [sh.status() :octicons-link-external-16:](https://www.mongodb.com/docs/manual/reference/method/sh.status/){:target="_blank"}.

docs/sharding.md:47

  • Matching the initial boundaries is not what makes the source balancer safe: this page later states that source migrations are not replicated and that the layouts can diverge. Reads and writes remain safe during migrations because they go through mongos and are routed using each cluster's current metadata.
The target starts from the same chunk boundaries as the source, so a chunk migration on the source arrives where the target expects it. That makes it safe to leave the balancer running during the sync, which matters in write-heavy clusters where turning it off is not an option. See [Manage sharded cluster balancer :octicons-link-external-16:](https://www.mongodb.com/docs/manual/tutorial/manage-sharded-cluster-balancer/){:target="_blank"} in the MongoDB documentation.

docs/sharding.md:55

  • Recreating boundaries does not ensure every target shard receives writes or that no rebalancing follows. For example, two source chunks cannot populate five target shards without adding boundaries, and indivisible uneven chunks can still require later balancing.
{{pcsm.short}} therefore recreates the source chunk boundaries on the target before copying any documents, so the clone writes to every shard from the start and no rebalancing wave follows. Matching boundaries are also what makes it safe to leave the balancer running on the source, as described in [Balancer operation](#balancer-operation).

docs/sharding.md:101

  • These recovery instructions contradict docs/limitations.md:56, which says an initial synchronization failure cannot be resumed and requires starting over. Since pre-splitting occurs before document copying, clarify which behavior applies and update the limitation if resume --from-failure now handles this stage.
A failed pre-split fails the clone for that instance, and there is no fallback to loading into an unsplit collection. Check the log for the reported failure, resolve it on the target cluster, then restart replication with `pcsm resume --from-failure`. See [Resume the replication](install/usage.md#resume-the-replication), [Logging in {{pcsm.full_name}}](logging.md), and the [Troubleshooting guide](troubleshooting.md).
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/sharding.md Outdated
Comment thread docs/sharding.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: rasika-chivate <95711051+rasika-chivate@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Multiple moderate documentation inaccuracies must be corrected before approval.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

docs/sharding.md:108

  • This example fails for valid collection names that are not JavaScript identifiers, such as names containing hyphens. Use getCollection() so replacing the placeholder always produces valid mongosh syntax.

docs/sharding.md:41

  • The new behavior still conflicts with the current contract: line 39 says only the shard key is preserved, while docs/limitations.md:31-34 says chunk distribution is not preserved and links to the nonexistent sharding.md#limitations anchor. Update both locations to distinguish copying the initial ranged-key layout from replicating subsequent sharding metadata, and point the limitations link to #chunk-distribution.
For a ranged shard key, immediately after it shards a collection on the target and before copying any documents, {{pcsm.short}} pre-splits the collection using the source chunk boundaries. Hashed collections retain the layout created by `shardCollection`. See [Chunk distribution](#chunk-distribution).

docs/sharding.md:47

  • This applies matching boundaries to every collection even though lines 41 and 71 state that hashed layouts are not copied, and it implies source migrations are propagated even though line 67 says later migrations have no effect on the target. Clarify that only ranged keys start with copied boundaries and that independent metadata migrations are not replicated; routing through mongos, rather than matching boundaries, is what allows the balancers to remain active.
The target starts from the same chunk boundaries as the source, so a chunk migration on the source arrives where the target expects it. That makes it safe to leave the balancer running during the sync, which matters in write-heavy clusters where turning it off is not an option. See [Manage sharded cluster balancer :octicons-link-external-16:](https://www.mongodb.com/docs/manual/tutorial/manage-sharded-cluster-balancer/){:target="_blank"} in the MongoDB documentation.

docs/sharding.md:55

  • Recreating the source boundaries does not guarantee that the clone writes to every target shard: the source may have fewer chunks than the target has shards, or its chunks may occupy only a subset of shards. The target balancer can also still migrate chunks, as lines 67 and 111 acknowledge, so “no rebalancing wave” is not guaranteed; describe this as reducing the initial concentration and rebalancing work instead.
{{pcsm.short}} therefore recreates the source chunk boundaries on the target before copying any documents, so the clone writes to every shard from the start and no rebalancing wave follows. Matching boundaries are also what makes it safe to leave the balancer running on the source, as described in [Balancer operation](#balancer-operation).

docs/sharding.md:57

  • “This” refers to recreating source boundaries, but that operation does not run for hashed collections according to lines 41 and 71. Restrict the statement to ranged shard keys so readers do not expect hashed collections to be pre-split from the source layout.
This runs automatically for every sharded collection, immediately after {{pcsm.short}} shards it on the target. There is no flag and nothing to configure.

docs/sharding.md:79

  • “Every shard” and “roughly the same volume” cannot be guaranteed. If the source has one ranged chunk and the target has three shards, preserving the boundaries leaves only one assignable chunk; similarly, a single oversized indivisible chunk can keep totals highly uneven. Describe the placement as best effort and explicitly allow empty target shards.
With different shard counts, the boundaries still come from the source, but the largest chunks are placed first, each on whichever target shard holds the least data at that point. Every shard ends up owning chunks and holding roughly the same volume. The estimate carries across collections, so the large chunks of several collections do not all collect on one shard.

docs/sharding.md:111

  • Requiring data on every shard can falsely indicate a failed pre-split when there are fewer source chunks than target shards; preserving boundaries legitimately leaves some shards empty in that case. Direct readers to inspect the copied boundaries and resulting distribution without using nonempty ownership as the success criterion.
Look for data on every shard rather than an exact match with the source, since counts differ even immediately after the clone and keep changing as the balancer works. For chunk counts per shard across the cluster, use [sh.status() :octicons-link-external-16:](https://www.mongodb.com/docs/manual/reference/method/sh.status/){:target="_blank"}.
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Several moderate documentation contradictions and invalid distribution guarantees remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

docs/sharding.md:15

  • MongoDB assigns the primary shard at the database level, not per collection. This wording misstates the sharding model and is repeated in docs/limitations.md:33; describe the database's primary shard in both places.
{{pcsm.short}} replicates data and not sharding metadata. For a collection with a ranged shard key it copies the initial chunk boundaries to the target before the clone starts, but it does not replicate any sharding metadata changes that follow, and the primary shard name for a collection may differ on source and target clusters. See [Chunk distribution](#chunk-distribution).

docs/sharding.md:39

  • Ownership is not copied when source and target shard counts differ: line 76 says chunks are reassigned using estimated volume. Limit this claim to equal shard counts or state that target ownership is initialized according to the target shard count.
Before starting the initial sync, {{pcsm.short}} checks which collections are sharded on the source cluster and creates corresponding sharded collections on the destination cluster. The sharding key is preserved from the source cluster. For ranged shard keys, {{pcsm.short}} also copies the initial chunk boundaries and ownership to the target; it does not replicate sharding metadata afterwards.

docs/sharding.md:41

  • The main workflow remains contradictory: docs/intro.md:117 says the target manages every detail other than the shard key, and line 127 proceeds directly from shard-key setup to copying data. Update that workflow to include the new ranged-key pre-split step so readers do not receive different startup behavior depending on the page.
For a ranged shard key, {{pcsm.short}} then pre-splits the collection using the source chunk boundaries, immediately after it shards the collection on the target and before it copies any documents. Collections with a hashed shard key keep the layout that `shardCollection` creates. See [Chunk distribution](#chunk-distribution).

docs/sharding.md:76

  • This guarantee fails when the target has more shards than there are source chunks; chunk placement cannot give every target shard a chunk in that case. State that balancing is best-effort and explicitly allow shards to remain empty.
With different shard counts, the boundaries still come from the source, but the largest chunks are placed first, each on whichever target shard holds the least data at that point. Every shard ends up owning chunks and holding roughly the same volume. The estimate carries across collections, so the large chunks of several collections do not all collect on one shard.

docs/sharding.md:108

  • Data on every shard is not a valid success criterion. A target can have more shards than source chunks, and even an owned chunk can contain no documents, so a successful pre-split may legitimately show no data on some shards.
Look for data on every shard rather than an exact match with the source, since counts differ even immediately after the clone and keep changing as the balancer works. For chunk counts per shard across the cluster, use [sh.status() :octicons-link-external-16:](https://www.mongodb.com/docs/manual/reference/method/sh.status/){:target="_blank"}.

docs/sharding.md:66

  • This treats source resharding as harmless drift, but docs/limitations.md:37 says reshardCollection is unsupported, fails replication, and requires a new initial sync. Remove resharding from the expected-drift list and warn that it fails the replication.
    {{pcsm.short}} reads the source boundaries once, before the clone, and does not replicate sharding metadata afterwards. Later migrations, splits, merges, and resharding on the source have no effect on the target, so the two layouts drift apart as the balancers work. That is expected and does not indicate a replication problem.
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/limitations.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread docs/limitations.md Outdated
* {{pcsm.short}} replicates the data and doesn't replicate metadata. This means that the following information is not preserved from the source cluster:
* {{pcsm.short}} replicates the data and doesn't continuously replicate metadata. For ranged shard keys, the initial chunk boundaries are copied and ownership is initialized on the target during the initial sync, but subsequent sharding metadata changes are not replicated. The following information is therefore not preserved from the source cluster:

* The primary shard name for a collection. The target cluster may have a different primary shard name.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these points are not properly indented

Comment thread docs/sharding.md Outdated
Since {{pcsm.short}} connects through `mongos`, the cluster topology doesn't matter. This means the source and target clusters can have different numbers of shards.

Also, {{pcsm.short}} replicates data and not metadata. This means chunk distribution as well as the primary shard name for a collection may differ on source and target clusters.
{{pcsm.short}} replicates data and not sharding metadata. For a collection with a ranged shard key it copies the initial chunk boundaries to the target before the clone starts, but it does not replicate any sharding metadata changes that follow, and the primary shard name for a collection may differ on source and target clusters. See [Chunk distribution](#chunk-distribution).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first sentence is confusing. We say we don't replicate metadata but then say we do... I suggest rephrasing to make it clear we copy at the beginning but don't continuously replicate metadata changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants